Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix data-layout field in x86_64-unknown-linux-gnu.json test file #45108

Merged
merged 2 commits into from
Oct 10, 2017
Merged

Fix data-layout field in x86_64-unknown-linux-gnu.json test file #45108

merged 2 commits into from
Oct 10, 2017

Conversation

phil-opp
Copy link
Contributor

@phil-opp phil-opp commented Oct 8, 2017

The current data-layout causes the following error:

rustc: /checkout/src/llvm/lib/CodeGen/MachineFunction.cpp:151: void llvm::MachineFunction::init(): Assertion `Target.isCompatibleDataLayout(getDataLayout()) && "Can't create a MachineFunction using a Module with a " "Target-incompatible DataLayout attached\n"' failed.

The new value was generated according to this comment by @japaric.

@phil-opp
Fix data-layout field
e8ba32e 
The value was generated according to [this comment by @japaric](rust-lang#31367 (comment)).
@phil-opp phil-opp changed the title Fix data-layout field Fix data-layout field in x86_64-unknown-linux-gnu.json test file Oct 8, 2017
@carols10cents
Copy link
Member

r? @japaric

Thanks for the PR! We’ll periodically check in on it to make sure that @japaric or someone else from the team reviews it soon.

@carols10cents carols10cents added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 9, 2017
@japaric
Copy link
Member

japaric commented Oct 9, 2017

The change LGTM but the data-layout changed quite long time ago; why hasn't this break a test?

It seems to me that this file is not being used at all. When rustc sees --target $T it will use the built-in definition of $T if there is one; otherwise it will look for a specification file named $T.json. So I think that the test that is meant to use this file is actually using the built-in target definition. I think it would be better to rename the file to not match the name of a built-in target and then adjust the test accordingly.

Could you look into it @phil-opp? (TBH, I don't even know which test is supposed to be using this file.)

@phil-opp
Rename test Linux target to avoid conflict with built-in target
06b9168 
It seems like the file wasn't actually used, since there is a built-in target with the same name. See rust-lang#45108 (comment) for more details.
@phil-opp
Copy link
Contributor Author

phil-opp commented Oct 10, 2017 •
edited
Loading

the data-layout changed quite long time ago; why hasn't this break a test?

Yeah, that wondered me too. I think that the file should be used by the target-specs test (see the Makefile).

I added a commit that renames the file to my-x86_64-unknown-linux-gnu-platform.json (to match the other test targets) and adjusts the target in the Makefile.

(I couldn't run the test locally, since I only have my laptop with a small SSD at the moment.)

@japaric
Copy link
Member

japaric commented Oct 10, 2017

Thanks, @phil-opp.

Travis seems to be happy so let's land this.

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Oct 10, 2017

📌 Commit 06b9168 has been approved by japaric

@japaric japaric added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 10, 2017
@kennytm kennytm mentioned this pull request Oct 10, 2017
Merged
kennytm added a commit to kennytm/rust that referenced this pull request Oct 10, 2017
@kennytm
Rollup merge of rust-lang#45108 - phil-opp:patch-2, r=japaric
3b69c79 
Fix data-layout field in x86_64-unknown-linux-gnu.json test file

The current data-layout causes the following error:

> rustc: /checkout/src/llvm/lib/CodeGen/MachineFunction.cpp:151: void llvm::MachineFunction::init(): Assertion `Target.isCompatibleDataLayout(getDataLayout()) && "Can't create a MachineFunction using a Module with a " "Target-incompatible DataLayout attached\n"' failed.

The new value was generated according to [this comment by @japaric](rust-lang#31367 (comment)).
bors added a commit that referenced this pull request Oct 10, 2017
@bors
Auto merge of #45169 - kennytm:rollup, r=kennytm
d6d711d 
Rollup of 9 pull requests

- Successful merges: #44775, #45089, #45095, #45099, #45101, #45108, #45116, #45135, #45146
- Failed merges:
@bors bors merged commit 06b9168 into rust-lang:master Oct 10, 2017
@phil-opp phil-opp deleted the patch-2 branch October 13, 2017 08:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@japaric japaric

Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@phil-opp @carols10cents @japaric @bors